CS-10618: Implement bidirectional realm sync command#4419
Conversation
Add `boxel realm sync` for bidirectional syncing between a local directory and a Boxel realm. Uses a manifest-based approach to detect changes on both sides, with conflict resolution strategies (--prefer-local, --prefer-remote, --prefer-newest). Extracts shared sync-manifest utilities from push command. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use 'update' instead of 'add' in atomic upload when file exists on remote but no manifest is present (first sync with overlapping files) - Add 1s delay in establishBaseline to avoid same-second mtime collisions - Account for realm-default index.json in manifest assertions - Stabilize no-op test by syncing once before checking for no checkpoint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d0c7f70a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (totalOps === 0) { | ||
| console.log('\nEverything is up to date'); | ||
| if (!this.options.dryRun && !effectiveManifest) { | ||
| // First sync with no changes needed - still write manifest | ||
| await this.writeManifest(localHashes, remoteMtimes); |
There was a problem hiding this comment.
When conflicts are skipped (no --prefer-* strategy), totalOps can still be 0, so this branch reports “Everything is up to date” and writes a first-sync manifest. In that scenario, conflicting local/remote versions get recorded as if they were synchronized, so later runs classify them as unchanged and never surface the conflict again. Skip-success paths should not write a manifest while skippedConflicts is non-empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in da77e80 — added a skippedConflicts.length === 0 guard to the manifest write condition, so first-sync manifests are only written when there are no unresolved conflicts.
There was a problem hiding this comment.
Pull request overview
Adds a new boxel realm sync command to support bidirectional syncing between a local directory and a Boxel realm using a persisted sync manifest, with conflict-resolution flags and integration coverage.
Changes:
- Introduce
realm synccommand implementing bidirectional classification (push/pull/deletes/conflicts) and checkpointing. - Extract shared manifest helpers into
src/lib/sync-manifest.tsand refactorrealm pushto use them. - Add integration tests covering bidirectional sync, conflicts, deletion behavior, dry-run, and manifest updates.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/boxel-cli/src/commands/realm/sync.ts | New bidirectional sync command + conflict strategies, manifest update, and checkpoint creation. |
| packages/boxel-cli/src/lib/sync-manifest.ts | New shared utilities for reading/writing the .boxel-sync.json manifest and hashing files. |
| packages/boxel-cli/src/commands/realm/push.ts | Refactor to use shared sync-manifest utilities instead of inline implementations. |
| packages/boxel-cli/src/commands/realm/index.ts | Register the new sync subcommand under boxel realm. |
| packages/boxel-cli/tests/integration/realm-sync.test.ts | New integration test suite for sync behavior and conflict/deletion scenarios. |
| docs/cs-10618-realm-sync-plan.md | Design/plan document for the new sync command and test coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove plan doc from repo per reviewer feedback. Fix manifest being written on first sync when conflicts are skipped (which would silently lose conflict state). Add explicit conflict handling for cross-state combinations (changed+added) that previously fell through to noop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…etions Fall back to getRemoteFileList when _mtimes endpoint is unavailable so remote-only files remain visible for pulling. Preserve manifest entries for files deleted locally but not propagated (no --delete), preventing unexpected re-pulls on the next sync. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t remoteMtimes Derive localFiles from getLocalFileListWithMtimes() instead of calling both getLocalFileList() and getLocalFileListWithMtimes() separately. Use manifest.files as a secondary known-paths set in classifyRemote() so files known from a previous sync but lacking remoteMtimes are classified as 'changed' instead of 'added', avoiding false conflicts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move classifyLocal, classifyRemote, determineAction, and resolveConflict from private RealmSyncer methods into src/lib/sync-logic.ts as pure exported functions. Add 42 unit tests covering classification, action matrix (including cross-state conflicts fix), deletion semantics, and all conflict resolution strategies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
boxel realm synccommand for bidirectional syncing between a local directory and a Boxel realm--prefer-local,--prefer-remote,--prefer-newest--deleteflag to sync deletions and--dry-runfor previewing changesSyncManifest,computeFileHash,loadManifest,saveManifest,pathExists) from push command intosync-manifest.tsTest plan
realm syncintegration tests--prefer-local,--prefer-remote,--prefer-newest--dry-runpreviews without making changesrealm pushstill works after manifest extraction🤖 Generated with Claude Code